20260616-aes-fixes#10709
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10709
Scan targets checked: wolfcrypt-bugs, wolfcrypt-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
|
douzzer
force-pushed
the
20260616-aes-fixes
branch
2 times, most recently
from
a9fefc4 to
f1b8d03
Compare
Frauschi
left a comment
Frauschi
left a comment
There was a problem hiding this comment.
Should we add test coverage for the OVERFLOW_E cases?
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: COMMENT
Findings: 5 total — 4 posted, 1 skipped
4 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [Low] Comment typo: 'cunulative' should be 'cumulative' —
wolfcrypt/src/aes.c:12813,12967 - [Low] Brace-on-own-line and redundant double parentheses deviate from file style —
wolfcrypt/src/aes.c:12816-12821,12970-12975 - [Low] Bare 0xffffffff literal; prefer suffixed/named constant —
wolfcrypt/src/aes.c:12817-12818,12971-12972 - [Low] Reused AES_CCM_OVERFLOW_E error string is about 'invocation counter', not input length —
wolfcrypt/src/aes.c:13621,13785
Skipped findings
- [Medium]
No tests added for new AES-GCM/CCM overflow error paths
Review generated by Skoll
…esGcmEncryptUpdate(), wc_AesGcmDecryptUpdate(), wc_AesCcmEncrypt(), and wc_AesCcmEncrypt().
…"catch and error on total length overflow".
douzzer
force-pushed
the
20260616-aes-fixes
branch
from
f1b8d03 to
1070384
Compare
dgarske
left a comment
dgarske
left a comment
There was a problem hiding this comment.
Skoll Multi-Scan Review
Modes: reviewOverall recommendation: COMMENT
Findings: 3 total — 3 posted, 0 skipped
3 finding(s) posted as inline comments (see file-level comments below)
One or more scans did not complete — the results below are partial.
Posted findings
- [Low] [review] GCM aSz (AAD) overflow branch is not exercised by tests —
wolfcrypt/test/test.c:19152-19161,19179-19188 - [Low] [review] Raw sizeof in mixed-signedness comparison deviates from function's wordSz convention —
wolfcrypt/src/aes.c:13618,13782 - [Info] [review] Test code pointer style and line length deviate from wolfSSL conventions —
wolfcrypt/test/test.c:21128-21130
Review generated by Skoll
…c/port/riscv/riscv-64-aes.c, wolfcrypt/src/port/silabs/silabs_aes.c, wolfcrypt/src/port/ti/ti-aes.c: implement AES-CCM counter overflow checks for ports;
wolfcrypt/test/test.c: add missing !HAVE_SELFTEST gate around AES-CCM counter overflow test in aesccm_128_badarg_test();
wolfcrypt/src/error.c and wolfssl/wolfcrypt/error-crypt.h: update messages for AES_{GCM,CCM}_OVERFLOW_E.
…her than magic 0xffffffff; wolfcrypt/test/test.c: in aesgcm_stream_test(), implement tests for sSz overflow, and in aesccm_128_badarg_test(), fix line length.
|
Note latest push passed |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10709
Scan targets checked: wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
No new issues found in the changed files. ✅
|
Retest this please Jenkins. Generic config failed. |
|
retest this please |
JacobBarthelmeh
dismissed
Frauschi’s stale review
Test case was added and feedback was addressed.
6 participants
wolfcrypt/src/aes.c: catch and error on total length overflow inwc_AesGcmEncryptUpdate(),wc_AesGcmDecryptUpdate(),wc_AesCcmEncrypt(), andwc_AesCcmEncrypt().tested with